Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip empty label fields #2019

Merged
merged 1 commit into from Feb 24, 2019
Merged

Skip empty label fields #2019

merged 1 commit into from Feb 24, 2019

Conversation

FroggyFlox
Copy link
Member

Add simple if statement to ignore fields with empty values for labels.

Fixes #2018.

Add simple if statement to ignore fields with empty values for labels.

Fixes rockstor#2018.
@phillxnet
Copy link
Member

@FroggyFlox Thanks for the quick fix, well done. Tested via custom RPM build rev 48.2019.

By way of a quick functional test we have entry of an empty value:

request-multiple-labels-one-empty

resulting in that empty value being ignored in Web-UI:

no-empty-web-ui-or-db-counterpart-results

with of course the mostly redundant double check on the storageadmin_dcontainerlabel db table:

storageadmin_dcontainerlabel

Best we move towards input validation as we go along, and as it's spec emerges in docker, but this avoids an easy inadvertent null db entry for now which would only come back to haunt us.

Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed as per prior comments.

Thanks for getting this sorted so quick.

@phillxnet phillxnet merged commit 8384681 into rockstor:master Feb 24, 2019
@FroggyFlox
Copy link
Member Author

Thanks a lot!
Sorry again I (somehow) missed something like that and had to make a patch.

@phillxnet
Copy link
Member

@FroggyFlox No worries at all. This is just part of the practicality of what we do and we just have to muddle through, oh and try and review each others code of course :). Speaking of which I'm wrapping up the revision 48 release now so we need to review all that's going into 49. Any chance you could glance over my pending pr. Not in the Rock-ons area but it's almost all Python so there's a bonus.

@FroggyFlox FroggyFlox deleted the patch-1 branch December 23, 2019 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants